[Dynamic Dashboard] Orders card status filtering #11533
Conversation
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #11533 +/- ##
============================================
- Coverage 40.45% 40.41% -0.04%
Complexity 5181 5181
============================================
Files 1082 1082
Lines 62904 62966 +62
Branches 8616 8621 +5
============================================
Hits 25447 25447
- Misses 35169 35231 +62
Partials 2288 2288 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Nice work @0nko 💯
I left two np comments, but they are not blocking.
| } | ||
|
|
||
| private val orderStatusMap = MutableSharedFlow<Map<String, Order.OrderStatus>>(replay = 1) | ||
| private val refreshTrigger = MutableSharedFlow<RefreshEvent>(extraBufferCapacity = 1) | ||
| private val statusOptions = MutableSharedFlow<List<OrderStatusOption>>(replay = 1) |
There was a problem hiding this comment.
np, given the usage of a SharedFlow here, if we don't have any cached values for the order status options, we won't show emit the UI state until the API call to fetch them is done.
WDYT about replacing the MutableSharedFlow with a MutableStateFlow that starts with listOf(DEFAULT_FILTER_OPTION...)? This way we'll start with all as a single entry at the beginning.
There was a problem hiding this comment.
That's a clever idea 👍
| val viewState = selectedFilter.flatMapLatest { status -> | ||
| refreshTrigger.map { Pair(status, it) } | ||
| }.transformLatest { (filterStatus, refresh) -> | ||
| if (refresh.isForced) { |
There was a problem hiding this comment.
np, wouldn't the check for forced here cause issues on initial load if we don't have any cached orders? I think that as we won't emit anything, we won't have any UI until the orders fetch is done, or I am missing something 🤔
There was a problem hiding this comment.
Hm, I added it to avoid the ugly flashing when changing the status filter. I added a check for this.
0nko
changed the title
Part of #11461, which implements the order filtering by status.
Screen_recording_20240516_212933.webm
To test:
All